upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences #10625

nikhagra-akamai · 2024-07-01T10:42:52Z

Description 📝

Added a feature to save the users global filter selection to preferences & load it back when user revisit the page.

Changes 🔄

Added UserPreference utility file to handle all the preferences related operations
Modified all the global filter components to by default get selected with preferences value, if any present.

Target release date 🗓️

Please specify a release date to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.

Preview 📷

Include a screenshot or screen recording of the change

💡 Use <video src="" /> tag when including recordings in table.

Before	After

How to test 🧪

Note: Since Dashboards api is not ready to use, therefore have to enable mock services for testing. But with mock services preferences will not be saved properly, that's why it can be verified from the network tab that preferences api call is going on initial loading of the CloudPulse page to retrieve the saved preferences as well as on changing of every global filter value to update it.

Select values from global filter drop-downs.
Refresh the page or open in new tab or new window
You will see that your previous selections are retrieved from the preferences ( no saved preferences will be retrieved in case of mock user).

As an Author I have considered 🤔

Check all that apply

…rences

mjac0bs · 2024-07-01T14:51:25Z

@nikhagra Before we review this PR, can we get a complete PR description? Unless there are no visual changes, the Preview section is helpful to see the UI differences at a glance. Including steps in How to test allows us to verify we can reproduce your changes as expected and edge cases have been accounted for.

nikhagra-akamai · 2024-07-01T15:20:25Z

@nikhagra Before we review this PR, can we get a complete PR description? Unless there are no visual changes, the Preview section is helpful to see the UI differences at a glance. Including steps in How to test allows us to verify we can reproduce your changes as expected and edge cases have been accounted for.

@mjac0bs Yes working on it

venkymano-akamai

LGTM

bnussman-akamai

I worry that /v4/profile/preferences won't be a good long term solution for storing this stuff.

bnussman-akamai · 2024-07-02T16:00:55Z

packages/manager/src/features/CloudPulse/Utils/UserPreference.ts

We should be able to use our React Query data layer for anything related to user preferences. (packages/manager/src/queries/profile/preferences.ts). We try to use that instead of interfacing with @linode/api-v4 functions directly

To add to Banks, here's reference point in our docs: https://linode.github.io/manager/development-guide/05-fetching-data.html#react-query

Thanks!! for the suggestion. We will take this up as part of enhancements later to use React Query Data layer via https://track.akamai.com/jira/browse/DI-19503 as in Q3 we have a commitment for beta with minimum functionalities. Hope this is fine for now.

@kmuddapo Use of React Query is something we'd need to see in this PR to get it merged, rather than later. Using direct api-v4 functions is rare in our codebase, and it may be acceptable on a one-off basis, but our best practice is to use RQ almost all the time -- this offers caching benefits, helps us manage states (loading, errors), as well as provides overall consistency in code. (See here for our section of documentation on this.) In this PR, even with debouncing, we'd be making several requests to /profile/preferences and we should be leveraging the RQ cache.

@nikhagra Please update to use React Query here in place of the api-v4 functions, and let us know if you run into any questions on implementing that switch.

github-actions · 2024-07-02T18:10:28Z

Coverage Report: ✅
Base Coverage: 82.31%
Current Coverage: 82.31%

mjac0bs

Thanks for the PR, @nikhagra. Left some comments throughout, and I do agree with Banks in thinking that a user's profile preferences doesn't feel like the best place to store CloudPulse prefs... were there other options discussed?

As far as the UI: I am seeing disabled fields for the dashboard, region, and resources. Should I be testing this with additional configurations on my account? I can interact with the fields if I turn on the Mock Service Worker via our dev tools, but that won't save the user's values.

packages/manager/.changeset/pr-10625-upcoming-features-1719830617170.md

packages/manager/src/features/CloudPulse/Utils/CloudPulseConstants.ts

mjac0bs · 2024-07-01T14:26:25Z

packages/manager/src/features/CloudPulse/Utils/UserPreference.ts

Let's rename this file utils.ts to remain consistent with convention in the codebase.

Changed the case of file name to camel case because in future more util files will be added for different functionalities

packages/api-v4/.changeset/pr-10625-upcoming-features-1719830627087.md

packages/api-v4/src/cloudpulse/types.ts

packages/manager/src/features/CloudPulse/shared/CloudPulseDashboardSelect.tsx

packages/manager/src/features/CloudPulse/shared/CloudPulseRegionSelect.tsx

packages/manager/src/features/CloudPulse/shared/CloudPulseDashboardSelect.tsx

packages/manager/src/features/CloudPulse/shared/CloudPulseRegionSelect.tsx

packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx

nikhagra-akamai · 2024-07-03T03:45:36Z

Thanks for the PR, @nikhagra. Left some comments throughout, and I do agree with Banks in thinking that a user's profile preferences doesn't feel like the best place to store CloudPulse prefs... were there other options discussed?

As far as the UI: I am seeing disabled fields for the dashboard, region, and resources. Should I be testing this with additional configurations on my account? I can interact with the fields if I turn on the Mock Service Worker via our dev tools, but that won't save the user's values.

Currently the endpoint for Dashboard component is not yet ready to use, that's why we have to use mock data for now.

…617170.md Co-authored-by: Mariah Jacobs <[email protected]>

…er components.

jcallahan-akamai · 2024-07-03T14:32:53Z

Currently the endpoint for Dashboard component is not yet ready to use, that's why we have to use mock data for now.

@nikhagra I have the same question as @mjac0bs. The first testing instruction you have provided is: "Select values from global filter drop-downs."

Yet all I see is disabled fields:

In your comment above you acknowledge that we must use mock data. If that is the case, please include steps to configure mock data appropriately in your testing instructions.

nikhagra-akamai · 2024-07-04T10:23:49Z

Currently the endpoint for Dashboard component is not yet ready to use, that's why we have to use mock data for now.

@nikhagra I have the same question as @mjac0bs. The first testing instruction you have provided is: "Select values from global filter drop-downs."

Yet all I see is disabled fields:

In your comment above you acknowledge that we must use mock data. If that is the case, please include steps to configure mock data appropriately in your testing instructions.

@jcallahan-akamai I've updated the testing instructions in the description. Since we can only test now as mock user, so preferences won't be saved for mock user but we can verify the same from network tab for preferences API call on value change.

mjac0bs

Thanks for addressing some initial feedback @nikhagra. Testing instructions are looking good now - appreciate the update. I confirmed the network requests and responses upon updating /profile/preferences.

Two more bits of feedback:

Please see my comment about React Query.
I am still somewhat concerned about storing these preferences in a user's profile preferences, rather than that information being returned from an aclp endpoint. Do we anticipate there being additional data stored in user preferences for CloudPulse?

As is, it feels out of place. For context, /profile/preferences is generally used to store things like what theme (light/dark/system) colors to use, or whether banners have been dismissed.

nikhagra-akamai · 2024-07-05T17:28:44Z

Thanks for addressing some initial feedback @nikhagra. Testing instructions are looking good now - appreciate the update. I confirmed the network requests and responses upon updating /profile/preferences.

Two more bits of feedback:

Please see my comment about React Query.

I am still somewhat concerned about storing these preferences in a user's profile preferences, rather than that information being returned from an aclp endpoint. Do we anticipate there being additional data stored in user preferences for CloudPulse?

As is, it feels out of place. For context, /profile/preferences is generally used to store things like what theme (light/dark/system) colors to use, or whether banners have been dismissed.

@mjac0bs

Sure will check.
More additional data will be stored in user preference for CloudPulse as we are going to add more features moving forward.

packages/manager/src/features/CloudPulse/shared/CloudPulseRegionSelect.tsx

packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx

carrillo-erik · 2024-07-09T13:29:56Z

@nikhagra Thanks for keeping up with all the PR feedback, I left some comments myself. It's almost there!

mjac0bs

Thanks Nikhil, approving pending a couple smaller bits of feedback are addressed, like naming the hooks as such and cleaning up another any.

Also left a few non-blocking nits about JSDocs comments, linter warnings, etc.

packages/manager/src/features/CloudPulse/Utils/UserPreference.ts

mjac0bs · 2024-07-09T15:45:05Z

packages/manager/src/features/CloudPulse/Utils/UserPreference.ts

Nice to have: JSDocs comments are useful to include for custom hooks and utils. useRestrictedGlobalGrantCheck and usePagination are a couple examples of where we do this.

packages/manager/src/features/CloudPulse/Overview/GlobalFilters.tsx

packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx

mjac0bs · 2024-07-10T19:06:57Z

Confirmed the only test failure is on linodes/clone-linode.spec.ts, which has been flaky and unrelated to this PR. Going to merge.

upcoming: [DI-19328] - Added capability to save & retrieve user prefe…

f8ee586

…rences

nikhagra-akamai requested a review from a team as a code owner July 1, 2024 10:42

nikhagra-akamai requested review from mjac0bs and carrillo-erik and removed request for a team July 1, 2024 10:42

upcoming: [DI-19328] - Added capability to save & retrieve user prefe…

3b04a09

…rences

mjac0bs added the Cloud Pulse label Jul 1, 2024

upcoming: [DI-19328] - Test cases added

8a266d1

upcoming: [DI-19328] - Test cases updated

6b0dd47

kmuddapo approved these changes Jul 2, 2024

View reviewed changes

venkymano-akamai approved these changes Jul 2, 2024

View reviewed changes

bnussman-akamai changed the title ~~upcoming: [DI-19328] - Added capability to save & retrieve user preferences~~ upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences Jul 2, 2024

bnussman-akamai reviewed Jul 2, 2024

View reviewed changes

mjac0bs requested changes Jul 2, 2024

View reviewed changes

Update packages/manager/.changeset/pr-10625-upcoming-features-1719830…

6f6584c

…617170.md Co-authored-by: Mariah Jacobs <[email protected]>

nikhagra-akamai requested a review from a team as a code owner July 3, 2024 12:43

nikhagra-akamai requested review from jdamore-linode and removed request for a team July 3, 2024 12:43

upcoming: [DI-19328] - Remove empty select component from global filt…

afc40fc

…er components.

nikhagra-akamai force-pushed the feature/preferences branch from a546c49 to afc40fc Compare July 3, 2024 12:49

mjac0bs self-requested a review July 3, 2024 14:48

upcoming: [DI-19328] - Fixed module importing issue

44f0ebe

nikhagra-akamai requested a review from bnussman-akamai July 4, 2024 15:10

mjac0bs reviewed Jul 5, 2024

View reviewed changes

DI-19328: Added ReactQuery in user preferences

22693dd

nikhagra-akamai requested review from venkymano-akamai and mjac0bs July 9, 2024 13:10

DI-19328: Added ReactQuery in user preferences

20d8e78

carrillo-erik reviewed Jul 9, 2024

View reviewed changes

packages/manager/src/features/CloudPulse/shared/CloudPulseRegionSelect.tsx Outdated Show resolved Hide resolved

carrillo-erik reviewed Jul 9, 2024

View reviewed changes

packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx Outdated Show resolved Hide resolved

carrillo-erik reviewed Jul 9, 2024

View reviewed changes

packages/manager/src/features/CloudPulse/shared/CloudPulseResourcesSelect.tsx Outdated Show resolved Hide resolved

DI-19328: Modified type of state variable in cloude pulse select

10d5366

nikhagra-akamai requested a review from carrillo-erik July 9, 2024 13:57

mjac0bs approved these changes Jul 9, 2024

View reviewed changes

mjac0bs assigned nikhagra-akamai Jul 9, 2024

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Jul 9, 2024

carrillo-erik approved these changes Jul 9, 2024

View reviewed changes

VenkatMano approved these changes Jul 10, 2024

View reviewed changes

upcoming: [DI-19328] - Improved naming conventions

5d03d05

mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Jul 10, 2024

mjac0bs merged commit fe34137 into linode:develop Jul 10, 2024
18 of 19 checks passed

This was referenced Jul 17, 2024

Release v1.124.0 - release → staging #10691

Merged

Release v1.124.0 - staging → master #10700

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences #10625

upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences #10625

nikhagra-akamai commented Jul 1, 2024 •

edited

Loading

mjac0bs commented Jul 1, 2024

nikhagra-akamai commented Jul 1, 2024 •

edited

Loading

venkymano-akamai left a comment

bnussman-akamai left a comment

bnussman-akamai Jul 2, 2024

mjac0bs Jul 2, 2024

kmuddapo Jul 3, 2024 •

edited

Loading

mjac0bs Jul 5, 2024

github-actions bot commented Jul 2, 2024 •

edited

Loading

mjac0bs left a comment

mjac0bs Jul 1, 2024

nikhagra-akamai Jul 3, 2024

nikhagra-akamai commented Jul 3, 2024

jcallahan-akamai commented Jul 3, 2024

nikhagra-akamai commented Jul 4, 2024

mjac0bs left a comment

nikhagra-akamai commented Jul 5, 2024

carrillo-erik commented Jul 9, 2024

mjac0bs left a comment

mjac0bs Jul 9, 2024

nikhagra-akamai Jul 10, 2024

mjac0bs commented Jul 10, 2024

upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences #10625

upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences #10625

Conversation

nikhagra-akamai commented Jul 1, 2024 • edited Loading

Description 📝

Changes 🔄

Target release date 🗓️

Preview 📷

How to test 🧪

As an Author I have considered 🤔

mjac0bs commented Jul 1, 2024

nikhagra-akamai commented Jul 1, 2024 • edited Loading

venkymano-akamai left a comment

Choose a reason for hiding this comment

bnussman-akamai left a comment

Choose a reason for hiding this comment

bnussman-akamai Jul 2, 2024

Choose a reason for hiding this comment

mjac0bs Jul 2, 2024

Choose a reason for hiding this comment

kmuddapo Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

mjac0bs Jul 5, 2024

Choose a reason for hiding this comment

github-actions bot commented Jul 2, 2024 • edited Loading

mjac0bs left a comment

Choose a reason for hiding this comment

mjac0bs Jul 1, 2024

Choose a reason for hiding this comment

nikhagra-akamai Jul 3, 2024

Choose a reason for hiding this comment

nikhagra-akamai commented Jul 3, 2024

jcallahan-akamai commented Jul 3, 2024

nikhagra-akamai commented Jul 4, 2024

mjac0bs left a comment

Choose a reason for hiding this comment

nikhagra-akamai commented Jul 5, 2024

carrillo-erik commented Jul 9, 2024

mjac0bs left a comment

Choose a reason for hiding this comment

mjac0bs Jul 9, 2024

Choose a reason for hiding this comment

nikhagra-akamai Jul 10, 2024

Choose a reason for hiding this comment

mjac0bs commented Jul 10, 2024

nikhagra-akamai commented Jul 1, 2024 •

edited

Loading

nikhagra-akamai commented Jul 1, 2024 •

edited

Loading

kmuddapo Jul 3, 2024 •

edited

Loading

github-actions bot commented Jul 2, 2024 •

edited

Loading